Make history file optional #16
Conversation
ConsoleSession.java
Outdated
| @@ -280,7 +288,7 @@ public final void close() { | |||
| session.close(); | |||
| client.close(); | |||
| try { | |||
| historyFile.flush(); | |||
| history.flush(); | |||
There was a problem hiding this comment.
Given that we only flush once I think you can change the field history to be History rather than PersistentHistory and then in this try-catch you can just check if(history instanceof PeristentHistory) { ((PeristentHistory)history ).flush(); } , so that we don't need to create a fake empty class.
| this.consoleReader.setHistory(this.historyFile); | ||
| PersistentHistory history; | ||
| try { | ||
| File file = new File(HISTORY_FILE); |
There was a problem hiding this comment.
Here you can maybe use File.exists() rather than creating a history = new FlushableMemoryHistory(); in the catch block
There was a problem hiding this comment.
Or you can also keep the catch, but I think it's important that we log with a warning the fact that we are falling back to an in-memory history, so maybe just add in the catch:
LOG.warn("An in-memory history will be used due to exception raised while trying to access history file: ", e.getMessage());
history = new FlushableMemoryHistory();
It's not a huge deal in this case, but it's generally important to be transparent with what's going on, i.e. don't secretly handle exceptions without warning if we're changing the expected behaviour
There was a problem hiding this comment.
@marco-scoppetta I think we actually want to fallback on any case on not being able to create/open a file. How could I utilize File.exists if creating a new file triggers IOException?
## What is the goal of this PR? Fixes CI on `master` by: - upgrading to latest `@graknlabs_console` to include typedb/typedb-console#16 - always running the build remotely
…b#16) ## What is the goal of this PR? Fixes first part of typedb/typedb#4803 ## What are the changes implemented in this PR? Upgrades `@build_bazel_rules_nodejs` to a version that includes a fix to ```` DEBUG: /private/var/tmp/_bazel_<>/<>/external/bazel_skylib/lib.bzl:30:1: WARNING: lib.bzl is deprecated and will go away in the future, please directly load the bzl file(s) of the module(s) needed as it is more efficient. ````
What is the goal of this PR?
Fix #7
Previously, Grakn Console would crash in case history file could not be created (possible reasons are user's home directory being non-present or mounted read-only)
What are the changes implemented in this PR?
@graknlabs_protocolversion.flush()history object regardless of whether it's backed byFileHistoryorMemoryHistory